Skip to content

feat: [SVLS-8390] drop sampled-out traces from backend when compute_trace_stats_on_extension is enabled#1060

Merged
lym953 merged 8 commits intomainfrom
yiming.luo/drop-traces
Mar 9, 2026
Merged

feat: [SVLS-8390] drop sampled-out traces from backend when compute_trace_stats_on_extension is enabled#1060
lym953 merged 8 commits intomainfrom
yiming.luo/drop-traces

Conversation

@lym953
Copy link
Copy Markdown
Contributor

@lym953 lym953 commented Mar 4, 2026

Summary

  • When compute_trace_stats_on_extension is true, traces with sampling priority <= 0 (AUTO_DROP / USER_DROP) are now dropped and not flushed to the Datadog backend.
  • When compute_trace_stats_on_extension is false (the current default), traces are not dropped because all traces need to be sent to Datadog so trace stats can be computed.
  • Trace stats computation is unaffected: dropped traces are still processed through process_traces and their stats are forwarded to the stats concentrator.

Test plan

  • Passed the added unit test

Manual test

Steps

  • Add logging for trace payload flushed, and for traces being dropped
  • Build a layer and install it on a Lambda function
  • Invoke the function 6 times

Result

Logs show that

  • some traces were dropped.
  • the aws.lambda span for some of the 6 invocations were not flushed to Datadog.

🤖 Partially generated with Claude Code

lym953 and others added 5 commits March 4, 2026 16:15
…n_extension is enabled

When `compute_trace_stats_on_extension` is true, traces with a sampling
priority of <= 0 (`AUTO_DROP` or `USER_DROP`) are now dropped and not
flushed to the Datadog backend. Stats computation is unaffected: dropped
traces are still processed and their stats are sent to the concentrator.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…_processed_traces

Instead of partitioning raw traces and calling process_traces twice, the
sampling-out filtering now happens inside ServerlessTraceProcessor::process_traces.
The payloads_for_stats clone (for stats) is taken before the filter, so stats
always include sampled-out traces. The backend payload has sampled-out chunks
removed in-place.

send_processed_traces is simplified to a single process_traces call, with stats
computed before the backend send. The OwnedTracerHeaderTags upfront conversion
is also removed since only one process_traces call is needed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When compute_trace_stats_on_extension is true and all trace chunks are
filtered out due to sampling priority, process_traces now returns
Option::None for the SendDataBuilderInfo instead of constructing an
empty payload. Callers skip the backend send when None is received.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

/// Sentinel value used by `collect_pb_trace_chunks` when `_sampling_priority_v1` is absent.
const CHUNK_PRIORITY_NOT_SET: i32 = i8::MIN as i32;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor

@duncanista duncanista Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have these available in libdatadog by any chance?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Imported the const from libdatadog.

@lym953 lym953 changed the title feat: drop sampled-out traces from backend when compute_trace_stats_on_extension is enabled feat: [SVLS-8390] drop sampled-out traces from backend when compute_trace_stats_on_extension is enabled Mar 5, 2026
@lym953 lym953 marked this pull request as ready for review March 5, 2026 23:40
@lym953 lym953 requested a review from a team as a code owner March 5, 2026 23:40
@lym953 lym953 requested a review from shreyamalpani March 5, 2026 23:40
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the trace processing pipeline so that when compute_trace_stats_on_extension is enabled, sampled-out traces (sampling priority <= 0) are excluded from the backend flush while still being included in the local stats computation.

Changes:

  • Update TraceProcessor::process_traces to optionally return no backend payload when all chunks are sampled out.
  • Filter sampled-out trace chunks from the backend payload when compute_trace_stats_on_extension is enabled, while preserving unfiltered payloads for stats generation.
  • Add unit tests covering partial and full sampled-out dropping behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
bottlecap/src/traces/trace_processor.rs Filters sampled-out chunks from the backend payload (when configured), returns None when nothing remains to flush, and adds unit tests for these behaviors.
bottlecap/src/otlp/agent.rs Adapts OTLP ingestion path to handle an optional backend payload (skip enqueue when None).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +410 to +414
Some(SendDataBuilderInfo::new(
builder,
body_size,
owned_header_tags,
)),
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After filtering sampled-out chunks, body_size may significantly overestimate the actual payload that will be queued and batched (it’s still based on the pre-filter trace vector). Since SendDataBuilderInfo.size is used by TraceAggregator to enforce the 3.2MB batch limit, this can lead to unnecessarily small batches / extra flushes when many chunks are dropped. Consider recomputing/adjusting the size based on the filtered payload (or at least based on kept chunk/span counts) when compute_trace_stats_on_extension filtering is applied.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Will address it in a separate PR.

…bdd-trace-normalization

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lym953 lym953 merged commit dda9b26 into main Mar 9, 2026
41 of 46 checks passed
@lym953 lym953 deleted the yiming.luo/drop-traces branch March 9, 2026 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants